Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AADSuspectedBruteForce.yaml #3634

Closed
wants to merge 5 commits into from
Closed

AADSuspectedBruteForce.yaml #3634

wants to merge 5 commits into from

Conversation

samikroy
Copy link
Contributor

@samikroy samikroy commented Dec 7, 2021

Testing for #3601

cmaneiro
cmaneiro previously approved these changes Dec 7, 2021
relevantTechniques:
- T1110
query: |
let authenticationWindow = 24h;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this, we don't want timeframes embedded in queries when not needed as this breaks the hunting blade UX time slider features.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shainw Please check #3601

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

let successCodes = dynamic(["0", "50125", "50140", "70043", "70044"]);
let aadFunc = (tableName:string){
table(tableName)
| where TimeGenerated > ago(authenticationWindow)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove, per above.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shainw Please check #3601

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

let aadFunc = (tableName:string){
table(tableName)
| where TimeGenerated > ago(authenticationWindow)
| extend Activities = pack("datetime", TimeGenerated,"ResultEventId", ResultType , "AppDisplayName", AppDisplayName, "ResultDescription", ResultDescription ,"IpAddress", IPAddress, "DeviceDetail", todynamic(DeviceDetail), "Status", todynamic(DeviceDetail), "Locationdetails", todynamic(LocationDetails))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this pack is highly intensive on a workspace with a large amount of data. We should summarize with these fields directly and then pack what you want after the summarize.
I would not pack IPAddress so this can be used as an Entity.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shainw Please check #3601

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

| where TimeGenerated > ago(authenticationWindow)
| extend Activities = pack("datetime", TimeGenerated,"ResultEventId", ResultType , "AppDisplayName", AppDisplayName, "ResultDescription", ResultDescription ,"IpAddress", IPAddress, "DeviceDetail", todynamic(DeviceDetail), "Status", todynamic(DeviceDetail), "Locationdetails", todynamic(LocationDetails))
| extend FailureOrSuccess = iff(ResultType in (successCodes), "Success", "Failure")
| summarize StartTime = min(TimeGenerated), EndTime = max(TimeGenerated), SuccesEvents = make_list_if(Activities, ResultType in (successCodes)),FailureEvents = make_list_if(Activities, ResultType !in (successCodes)) , FailureCount = countif(FailureOrSuccess=="Failure"), SuccessCount = countif(FailureOrSuccess=="Success") by bin(TimeGenerated, authenticationWindow), UserDisplayName, UserPrincipalName, Type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bring thru ResourceId so it can be used as an entity.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shainw Please check #3601

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResourceId is yet to be a common a column for both the tables. Please suggest if any other column is applicable.

let aadSignin = aadFunc("SigninLogs");
let aadNonInt = aadFunc("AADNonInteractiveUserSignInLogs");
union isfuzzy=true aadSignin, aadNonInt
entityMappings:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add IP and ResourceId mappings, this is not specifically supported in the UX yet, but it likely will be in the future and we want to make it easy for users to migrate to this to a detection for their specific environment if so desired.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shainw Please check #3601

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResourceId is yet to be a common a column for both the tables. Please suggest if any other column is applicable.

@samikroy
Copy link
Contributor Author

@shainw - thank you for the review, have addressed all.
ResourceId is yet to be a common a column for both the tables. Please suggest if any other column is applicable.
@shainw & @cmaneiro - Can we please remove UserDisplayName in the summarize to optimize !

@cmaneiro
Copy link
Contributor

Please we should stop working in parallel, the original PR is #3601, as stated in the first comment of this PR this is just a testing made by @samikroy to address the situation with the automatic validations.

Regards

@samikroy
Copy link
Contributor Author

Please we should stop working in parallel, the original PR is #3601, as stated in the first comment of this PR this is just a testing made by @samikroy to address the situation with the automatic validations.

Regards

Sure @cmaneiro

@samikroy
Copy link
Contributor Author

Please we should stop working in parallel, the original PR is #3601, as stated in the first comment of this PR this is just a testing made by @samikroy to address the situation with the automatic validations.
Regards

Sure @cmaneiro

Thank you for highlighting @cmaneiro, could not capture your last comments real time.
Apologies for the inconvenience caused.

@shainw
Copy link
Contributor

shainw commented Dec 30, 2021

Closing as dupe of #3601

@shainw shainw closed this Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants